Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Migrated app.config.chain.getById() calls to newer trpc.community.getCommunity() + cleanup/state-refactor of related components #8779

Merged
merged 84 commits into from
Aug 14, 2024

Conversation

mzparacha
Copy link
Contributor

@mzparacha mzparacha commented Aug 7, 2024

Link to Issue

Closes: #8762

Description of Changes

  • Removed all app.config.chains.getById references in favor of newer trpc.community.getCommunity route
  • Refactored multiple frontend modules to cleanup/abstract/simplify logic relating to community state.
  • Refactored frontend chain init according to /bulkOffchain API merged in trpc.community.getCommunity route
  • Reduced usage of ChainInfo object usage

"How We Fixed It"

N/A

Test Plan

We have to ensure nothing that referenced app.config.chain.getById() broke

  • Test all community pages including gated/restricted/special-case pages/actions like snapshots/contests/admin-pages/token-gating/stakes for any regressions
  • Test non-community pages specially profile/feed/notifications page for any regressions
  • Test site admin page for any regressions

Deployment Plan

N/A

Other Considerations

The changes in this pr will lead to some more cleanup/refinement. Created these tickets to make those changes as a follow-up.

Copy link
Contributor

@masvelio masvelio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

half way through 😓
now going to do some manual testing

Copy link
Contributor

@masvelio masvelio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. getting this error on dashboard page load
[13:22:00.257] ERROR (index.ts): INTERNAL_SERVER_ERROR: [ZodError] subscription.getCommentSubscriptions: [
  {
    "code": "invalid_type",
    "expected": "object",
    "received": "null",
    "path": [
      0,
      "Comment",
      "Thread"
    ],
    "message": "Expected object, received null"
  }
] 
    err: {
      "type": "ZodError",
      "message": "[\n  {\n    \"code\": \"invalid_type\",\n    \"expected\": \"object\",\n    \"received\": \"null\",\n    \"path\": [\n      0,\n      \"Comment\",\n      \"Thread\"\n    ],\n    \"message\": \"Expected object, received null\"\n  }\n]",
      "stack":
          ZodError: [
            {
              "code": "invalid_type",
              "expected": "object",
              "received": "null",
              "path": [
                0,
                "Comment",
                "Thread"
              ],
              "message": "Expected object, received null"
            }
          ]
              at get error [as error] (file:///Users/marcin/Desktop/Projects/commonwealth/node_modules/.pnpm/[email protected]/node_modules/zod/lib/index.mjs:587:31)
              at ZodArray.parseAsync (file:///Users/marcin/Desktop/Projects/commonwealth/node_modules/.pnpm/[email protected]/node_modules/zod/lib/index.mjs:715:22)
              at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
              at async outputMiddleware (file:///Users/marcin/Desktop/Projects/commonwealth/node_modules/.pnpm/@[email protected]/node_modules/@trpc/server/dist/index.mjs:308:26)
              at async callRecursive (file:///Users/marcin/Desktop/Projects/commonwealth/node_modules/.pnpm/@[email protected]/node_modules/@trpc/server/dist/index.mjs:451:32)
              at async callRecursive (file:///Users/marcin/Desktop/Projects/commonwealth/node_modules/.pnpm/@[email protected]/node_modules/@trpc/server/dist/index.mjs:451:32)
              at async resolve (file:///Users/marcin/Desktop/Projects/commonwealth/node_modules/.pnpm/@[email protected]/node_modules/@trpc/server/dist/index.mjs:481:24)
              at async inputToProcedureCall (file:///Users/marcin/Desktop/Projects/commonwealth/node_modules/.pnpm/@[email protected]/node_modules/@trpc/server/dist/resolveHTTPResponse-2fc435bb.mjs:46:22)
              at async Promise.all (index 0)
              at async resolveHTTPResponse (file:///Users/marcin/Desktop/Projects/commonwealth/node_modules/.pnpm/@[email protected]/node_modules/@trpc/server/dist/resolveHTTPResponse-2fc435bb.mjs:182:37)
      "aggregateErrors": [
        {
          "type": "Object",
          "message": "Expected object, received null",
          "stack":
              
          "code": "invalid_type",
          "expected": "object",
          "received": "null",
          "path": [
            0,
            "Comment",
            "Thread"
          ]
        }
      ],
      "issues": [
        {
          "code": "invalid_type",
          "expected": "object",
          "received": "null",
          "path": [
            0,
            "Comment",
            "Thread"
          ],
          "message": "Expected object, received null"
        }
      ],
      "name": "ZodError"
    }
  1. this api cal looks weird (maybe it is ok but just want to double check)
    image

  2. Filtering by stake does not work

image

  1. getting stake error on community load
    image
    Actually nothing related to stake does not work, I can't see indicator in left sidebar

  2. infinite loader on create community flow when reserving namespace

image

  1. community I just created -> I am not admin of this community, cannot see admin capabilities section

image

  1. search throws backend errors
    image

@mzparacha
Copy link
Contributor Author

mzparacha commented Aug 13, 2024

Re: #8779 (review)

  1. This is on the master branch as well -- this is some "types" issue on the API side -- I believe Kevin is working on the fix in Fixed bug where unauthenticated users would get subscription errors in the console. #8833
  2. normal - trpc batches multiple requests (if different calls are made in a short timespan) -- the caches of responses are managed normally image
  3. Fixed in 7a050be
  4. Fixed in 4521354
  5. This is on the master branch as well - will create a new ticket to investigate - image
  6. Fixed in 6b3889a
  7. This seems to be a "types" issue with the get community route. Basically the community.network is typed with ChainNetwork, and ChainNetwork doesn't include all the networks that communities in db might have. Ex: Regen is not listed in ChainNetwork but exists in the Communities table, visiting the regen community would break the route http://localhost:8080/api/v1/community.getCommunity?batch=1&input=%7B%220%22%3A%7B%22id%22%3A%22regen%22%2C%22include_node_info%22%3Atrue%7D%7D. I found ~59 more networks in my local db not listed in ChainNetwork and suspect there are probably more in prod db. We should list all those in ChainNetwork or maybe change/remove the type.

@jnaviask jnaviask temporarily deployed to commonwealth-frick August 13, 2024 16:47 Inactive
@mzparacha
Copy link
Contributor Author

@masvelio Thanks for the detailed review. I addressed all the feedback in #8779 (comment)

Also cc: @jnaviask - point 7 in #8779 (comment) relates to the chain network issues we discussed.

Copy link
Contributor

@masvelio masvelio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Zod validation error on communities page
Kapture.2024-08-14.at.10.27.03.mp4

Tested issues I found yesterday, work well now, thanks for fixing them. Added couple small comments + found 1 issue with zod validation. Apart from that, looks good to me. Fantastic job!

@mzparacha
Copy link
Contributor Author

Re: #8779 (review)

  1. Zod validation error on communities page

This also relates to 7 in #8779 (comment)

cc: @masvelio @jnaviask

jnaviask and others added 2 commits August 14, 2024 14:33
@mzparacha
Copy link
Contributor Author

The 7 from #8779 (comment) was fixed in #8843 -- Thanks to @jnaviask

@jnaviask
Copy link
Collaborator

Member count seems broken on the /members page but otherwise this PR seems good to me.

Copy link
Collaborator

@jnaviask jnaviask left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Members issue looks like it's my machine and not part of this PR.

@jnaviask jnaviask merged commit 483f6d4 into master Aug 14, 2024
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement GET /community/:id in frontend
3 participants